Skip to content

feat(goal): let the model propose a Goal the user approves in a dialog - #10171

Merged
qqqys merged 19 commits into
QwenLM:mainfrom
qqqys:feat/propose-goal-tool
Sep 1, 2026
Merged

feat(goal): let the model propose a Goal the user approves in a dialog#10171
qqqys merged 19 commits into
QwenLM:mainfrom
qqqys:feat/propose-goal-tool

Conversation

@qqqys

@qqqys qqqys commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Adds a propose_goal core tool: the model proposes a session Goal, the user sees the full objective in an approval dialog, and only their approval sets it — exactly as /goal set would. Declining sets nothing; the model is told only that the Goal was not set and must not propose it again. The tool is registered beside get_goal / update_goal (so never for subagents), only in interactive terminal sessions (resolveInteractionMode === 'interactive' — never headless, and not yet in Web Shell or other ACP-driven sessions, which bypass the turn boundary that applies the approval) and only while the new goals.modelProposed setting is not "disabled".

The dialog reuses the generic info confirmation, so no host needs a new component: the objective is in the invocation description (the one field every host forwards — the Web Shell does not render an info prompt) and in the plain-text prompt together with a one-sentence explanation of what approving does. The invocation declares requiresUserInteraction(), so no permissions.allow rule, no skill allowedTools grant, and no approval mode — YOLO included, and AUTO_EDIT, which auto-approves info confirmations — can skip the dialog. Preconditions are checked before the dialog and again in execute(), because /goal can change the session while the dialog is open: plan mode, an untrusted folder, a session without Goal persistence, and an active Goal all refuse with guidance. An active Goal is never replaced from the tool (the model is told to hand over a /goal edit … / /goal set … line instead); a stopped Goal (paused, blocked, complete, usage-limited) is replaced through replace with its expected version, so a Goal that changed under the dialog is rejected rather than clobbered. The tool never dispatches from inside the turn: creating the Goal mid-turn would leave the proposing turn's remaining tool-result continuations without a Goal permit (the first end-to-end run surfaced exactly that as an An active Goal requires an exact turn permit error card). The approval is parked on Config (a set-once slot) and GeminiClient.sendMessageStream applies it at the turn's true terminal sites — after queued steer input, blocking Stop-hook continuations, and the next-speaker recursion have completed — asking the runtime to refuse if a Goal became active meanwhile; the runtime's broadcast then renders the Goal card and starts the first Goal turn right after the model's one-sentence acknowledgement. The approval is bound to the proposing turn's prompt id, so only that turn's own terminal boundary can apply it and any other frame — a Notification, Cron, or Teammate turn, the user's next query, a hook-blocked or recursive frame — drops it on sight; an abort drops it too (and pauses a Goal an abort raced past), and every early-exit path discards it as a fast path.

goals.modelProposed ("alwaysAsk" default, "disabled") is added to the settings schema and to WORKSPACE_RESTRICTED_SETTINGS, so a repository's .qwen/settings.json cannot switch the tool on; the CLI normalizes the value to the closed enum before it reaches core. The bundled goal-draft skill now hands off through propose_goal when it is available and no Goal is active, and keeps its printed /goal set line for headless runs, the disabled setting, and the active-Goal case. Docs (goals.md, settings.md, the design note) and the Web Shell tool-name tables are updated.

Why it's needed

#10002 shipped /goal-draft, which ends by printing a /goal set … line for the user to copy and run — because built-in commands are not model-invocable, and deliberately so: starting an unattended loop is the user's decision. That hand-off works but is clumsy (a 1,000-character line to paste, and on some terminals the backticks get escaped), and it is the one place where the model's careful draft can be lost. Claude Code solves this with ProposeGoal + a modelProposedGoals setting: the model proposes, the user approves with one keypress. This PR is the qwen-code equivalent, with two deliberate differences: there is no auto mode that lets the model skip the dialog, and the tool never replaces a running Goal.

Reviewer Test Plan

How to verify

  1. cd packages/core && npx vitest run src/goals src/core/client-goal.test.ts src/config/config.test.ts src/skills/bundled/goal-draft/SKILL.test.ts and cd packages/cli && npx vitest run src/config/settings.test.ts src/config/config.test.ts — all pass. New cases: ProposeGoalTool (dialog contents, approval parks the proposal and applyPendingGoalProposal then sets the Goal and the runtime starts the first Goal turn, cancel parks nothing, refuses in plan mode / untrusted / no persistence / active Goal, replaces a paused Goal, refuses over a Goal that became active meanwhile, reports a moved version as a conflict, validation), the client boundary (a parked approval is applied once a user turn ends without tool calls; one still parked at the next user query is discarded), registration (present in interactive/ACP/stream-json, absent headless and when disabled), goals.modelProposed scope (honored from user scope, stripped from workspace scope with a warning), and the CLI normalizer.
  2. TUI, Ask mode, in a trusted project with a test script: /goal-draft make the auth tests pass and raise coverage. Expected: after drafting, a confirmation card Propose Goal: Outcome: … with the objective and "Set this as the session Goal? Approving sets it like /goal set …", options "Yes, allow once" / "No" only (no "Always allow"). Enter → Goal card create, the footer Goal pill appears, and the first Goal turn starts on its own; /goal shows revision 1 with the proposed objective.
  3. Same, but press Esc on the dialog: no Goal is set, /goal reports none, and the model does not re-propose.
  4. YOLO (-y) interactive: the dialog still appears. Plan mode: the tool returns "Keep planning; propose the Goal after the plan is approved." With a Goal active: the model prints a /goal edit … line instead.
  5. qwen -p "/goal-draft …" (headless): unchanged — propose_goal is not registered and the /goal set line is printed.
  6. Put "goals": {"modelProposed": "disabled"} in ~/.qwen/settings.json: the tool disappears (the skill falls back to the printed line). Put the same in .qwen/settings.json of a project: a startup warning says the value is ignored.

Evidence (Before & After)

TUI in YOLO mode (this machine's default), trusted throwaway project with test / coverage scripts, driven in tmux (160×48) against DashScope qwen3.8-max; screenshots are the tmux pane rendered with its own colors.

Before (main + #10002): /goal-draft ends with a /goal set … line to copy; there is no propose_goal tool.

After — 1. The skill's clarifying questions still come as a real dialog in YOLO (that is #10160), and the answers feed the draft.

After — 2. propose_goal renders the objective and the one-sentence explanation in the approval dialog. Only "Yes, allow once" / "No" are offered — no "Always allow", even in YOLO.

After — 3. Enter → the tool card reports Goal approved ·, the model acknowledges in one sentence and stops, and at that boundary the runtime sets the Goal: the Goal active card appears and the footer pill reads /goal active. No An active Goal requires an exact turn permit error card (the first end-to-end run, which set the Goal from inside the tool, produced one right here — hence the boundary application).

After — 4. The runtime drives the first Goal turn on its own (here: diagnosing the failing node --test test/ invocation) while the pill counts up.

On the first end-to-end run the same loop ran to the end: the runtime installed c8, added the missing-branch tests, pasted both check lines, and the independent verifier accepted the completion (Goal complete · 1 turn · 3m 57s). Cancelling the dialog with Esc leaves /goal empty (unit-tested; the scheduler cancels the call before execute, and execute itself parks nothing without approval).

Tested on

OS Status
🍏 macOS ⚠️
🪟 Windows ⚠️
🐧 Linux

Environment (optional)

Linux, Node 22, npm ci + npm run build + npm run bundle; TUI driven in tmux (160×50) against DashScope qwen3.8-max.

Risk & Scope

  • Main risk or tradeoff: the dialog is the generic info confirmation, so its buttons read "Yes, allow once" / "No, suggest changes" in the TUI and "Allow" / "Reject" in the Web Shell and ACP clients rather than "Set this goal" / "Not now"; the prompt text explains what approving does. A dedicated confirmation type would need rendering in all three hosts and is left for later. The tool never replaces an active Goal — replacing mid-run would preempt the running Goal turn from inside one of its own tool calls.
  • Not validated / out of scope: Web Shell and ACP dialogs are covered by the existing info path and unit tests, not by a new screenshot; parseGoalCommand still flattens newlines (separate follow-up); no auto mode.
  • Breaking changes / migration notes: none. A new setting with a safe default; the skill's printed hand-off remains wherever the tool is absent.

Linked Issues

Follow-up to #10002 (P2 of the goal-draft design note).

中文说明

这个 PR 做了什么

新增核心工具 propose_goal:模型提议一个会话 Goal,用户在审批对话框里看到完整 objective,只有用户批准才会设定——效果与 /goal set 完全一致。拒绝则什么都不设;模型只被告知"Goal 未设定",且不得再次提议。该工具与 get_goal / update_goal 一起注册(因此子 agent 永远没有),仅在交互式终端会话(resolveInteractionMode === 'interactive'——headless 没有,Web Shell 及其他 ACP 驱动的会话也暂时没有,它们不经过应用提案的 turn 边界)、且新设置 goals.modelProposed 不为 "disabled" 时才注册。

对话框复用通用的 info 确认类型,三端都不需要新组件:objective 同时放在调用描述里(这是每个宿主都会转发的唯一字段——Web Shell 不渲染 info 的 prompt)和纯文本 prompt 里,并附一句"批准意味着什么"的说明。调用声明了 requiresUserInteraction(),所以任何 permissions.allow 规则、skill 的 allowedTools 授权、任何审批模式——包括 YOLO,以及会自动批准 info 确认的 AUTO_EDIT——都跳不过对话框。前置条件在弹窗前检查一次、execute() 里再检查一次,因为对话框打开期间 /goal 可能改变会话:plan 模式、不受信任的目录、没有 Goal 持久化的会话、以及已有活跃 Goal,都会带指引拒绝。活跃 Goal 永远不会被这个工具替换(模型被要求改为给出 /goal edit … / /goal set … 行);已停止的 Goal(paused、blocked、complete、usage-limited)通过带期望版本的 replace 替换,所以在对话框下被改动过的 Goal 会被拒绝而不是被覆盖。工具从不在 turn 内部 dispatch:在 turn 中途创建 Goal 会让本 turn 剩余的工具结果续跑失去 Goal permit(第一次端到端运行正是暴露了 An active Goal requires an exact turn permit 这个错误卡片)。批准被暂存在 Config 上(set-once 槽),由 GeminiClient.sendMessageStream 在 turn 真正的终止点(排队的 steer、阻塞 Stop hook 续跑、next-speaker 递归都结束之后)应用,并要求 runtime 在期间出现活跃 Goal 时拒绝;随后 runtime 的广播渲染 Goal 卡片,并在模型的一句话确认之后紧接着启动首个 Goal turn。批准绑定提议 turn 的 prompt id:只有该 turn 自己的终止点能应用它,任何其它 frame——Notification/Cron/Teammate turn、用户的下一条消息、被 hook 拦截或递归的 frame——一旦碰到就丢弃;中止同样丢弃(若与 dispatch 竞争则暂停新 Goal),各提前退出路径的丢弃保留为快路径。

goals.modelProposed(默认 "alwaysAsk",可选 "disabled")加入设置 schema 和 WORKSPACE_RESTRICTED_SETTINGS,仓库的 .qwen/settings.json 无法打开这个工具;CLI 会先把值规范到封闭枚举再传给 core。内置 goal-draft skill 现在在工具可用且没有活跃 Goal 时通过 propose_goal 交接,在 headless、设置禁用、或已有活跃 Goal 时保留打印 /goal set 行的方式。文档(goals.mdsettings.md、设计说明)和 Web Shell 的工具名表已更新。

为什么需要

#10002 交付的 /goal-draft 以打印一行 /goal set … 让用户复制运行收尾——因为内置命令不可被模型调用,而且这是刻意的:启动无人值守循环应由用户决定。这个交接能用但笨拙(一行 1000 字符要粘贴,某些终端还会转义反引号),也是模型精心起草的内容最容易丢失的地方。Claude Code 用 ProposeGoal + modelProposedGoals 设置解决:模型提议,用户一键批准。本 PR 是 qwen-code 的对应实现,有两处刻意的差异:没有让模型跳过对话框的 auto 模式;工具永远不替换运行中的 Goal。

Reviewer 测试计划

如何验证

  1. cd packages/core && npx vitest run src/goals src/core/client-goal.test.ts src/config/config.test.ts src/skills/bundled/goal-draft/SKILL.test.tscd packages/cli && npx vitest run src/config/settings.test.ts src/config/config.test.ts——全部通过。新增用例:ProposeGoalTool(对话框内容、批准后暂存提案且 applyPendingGoalProposal 随后设定 Goal 并由 runtime 启动首个 Goal turn、取消不暂存、plan 模式 / 未信任 / 无持久化 / 活跃 Goal 时拒绝、替换 paused Goal、期间变为活跃则拒绝、版本移动报冲突、参数校验),client 边界(用户 turn 无工具调用结束时应用暂存的批准;下一条用户消息开始时仍暂存的被丢弃),注册(交互/ACP/stream-json 存在,headless 与禁用时不存在),goals.modelProposed 作用域(user 层生效,workspace 层剥离并告警),以及 CLI 的规范化函数。
  2. TUI、Ask 模式、受信任且带 test 脚本的项目:/goal-draft make the auth tests pass and raise coverage。预期:起草后出现确认卡片 Propose Goal: Outcome: …,带 objective 和"Set this as the session Goal? Approving sets it like /goal set …",选项只有 "Yes, allow once" / "No"(没有 "Always allow")。回车 → Goal 卡片 create、底部 Goal pill 出现、首个 Goal turn 自动开始;/goal 显示 revision 1 与提议的 objective。
  3. 同上但在对话框按 Esc:没有 Goal 被设定,/goal 报告无,模型不会再提议。
  4. YOLO(-y)交互:对话框仍然出现。Plan 模式:工具返回 "Keep planning; propose the Goal after the plan is approved."。已有活跃 Goal:模型改为打印 /goal edit … 行。
  5. qwen -p "/goal-draft …"(headless):不变——propose_goal 未注册,仍打印 /goal set 行。
  6. ~/.qwen/settings.json"goals": {"modelProposed": "disabled"}:工具消失(skill 回退到打印行)。在项目 .qwen/settings.json 写同样内容:启动时告警该值被忽略。

证据(Before & After)

Before:/goal-draft 以一行待复制的 /goal set … 结束;没有 propose_goal 工具。

After:见上方英文部分的 5 张 tmux 截图——skill 提问对话框(YOLO 下仍弹出)、propose_goal 审批对话框(只有 Yes/No,无 Always allow)、批准后 Goal approved · → 模型一句话确认 → 边界处 Goal active 卡片与底部 /goal active pill、以及 runtime 自行驱动首个 Goal turn;没有再出现 An active Goal requires an exact turn permit 错误卡片。首次端到端运行中同一循环跑到了 Goal complete · 1 turn · 3m 57s

测试平台

Linux ✅;macOS、Windows ⚠️ 未测试。

环境(可选)

Linux,Node 22,npm ci + npm run build + npm run bundle;TUI 在 tmux(160×50)中针对 DashScope qwen3.8-max 驱动。

风险与范围

  • 主要风险/取舍:对话框是通用 info 确认,TUI 里按钮文案是 "Yes, allow once" / "No, suggest changes",Web Shell 与 ACP 客户端里是 "Allow" / "Reject",而不是 "Set this goal" / "Not now";prompt 文本解释了批准的含义。专用确认类型需要三端各自渲染,留待后续。工具永远不替换活跃 Goal——运行中替换会从 Goal 自己的工具调用内部抢占正在运行的 Goal turn。
  • 未验证/范围外:Web Shell 与 ACP 的对话框由现有 info 路径和单元测试覆盖,没有新截图;parseGoalCommand 仍会压平换行(另行跟进);没有 auto 模式。
  • 破坏性变更/迁移说明:无。新设置有安全默认值;工具不存在的地方 skill 仍打印交接行。

关联 Issue

#10002 的后续(goal-draft 设计说明的 P2)。

Adds a propose_goal core tool: the model proposes a session Goal, the
user sees the full objective in an approval dialog, and only their
approval sets it, exactly as /goal set would. Declining sets nothing and
the model is told only that the Goal was not set. The tool is registered
beside get_goal/update_goal (never for subagents), only where a dialog
can be shown (interactive, ACP, stream-json; never plain headless) and
only while goals.modelProposed is not "disabled".

The dialog reuses the generic info confirmation, so no host needs a new
component: the objective is in the invocation description (the one field
every host forwards) and in the plain-text prompt with a one-sentence
explanation. requiresUserInteraction() is true, so no allow rule, skill
grant, YOLO, or AUTO_EDIT (which auto-approves info confirmations) can
skip the dialog. Preconditions are checked before the dialog and again in
execute(), because /goal can change the session while it is open: plan
mode, an untrusted folder, no Goal persistence, and an active Goal refuse
with guidance. An active Goal is never replaced from the tool; a stopped
Goal is replaced through `replace` with its expected version. The tool
only dispatches: the runtime's broadcast renders the card and starts the
first Goal turn after the current turn ends.

goals.modelProposed (alwaysAsk | disabled) joins WORKSPACE_RESTRICTED_SETTINGS
so a repository cannot switch the tool on; the CLI normalizes it to the
closed enum. The goal-draft skill hands off through propose_goal when it is
available and no Goal is active, and keeps its printed /goal set line for
headless runs, the disabled setting, and the active-Goal case.

Follow-up to QwenLM#10002 (P2 of the goal-draft design note).

Claude-Session: https://claude.ai/code/session_01FV7i3w7egJ2kMw4AhQC38Z
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

⚠️ Deferred approval withheld — 1 PR CI workflow run(s) on e2c77fd did not finish green; see the updated table in the Stage 2 comment. Re-run @qwen-code /triage after fixes. finalize run

⚠️ 延迟审批已搁置 —— e2c77fd 有 1 个 PR CI workflow 未以绿色完成,详见 Stage 2 评论中已更新的表格。修复后可重新运行 @qwen-code /triage查看 finalize 运行

… columns

The first cut used a quoted default that widened the Default column, so
prettier re-aligned 46 unrelated rows. The row now fits the existing
widths and the table diff is the one added line.

Claude-Session: https://claude.ai/code/session_01FV7i3w7egJ2kMw4AhQC38Z
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: real and observed. #10002 shipped /goal-draft with a paste-a-1,000-character-line hand-off, and the merged design doc for it explicitly lists this propose_goal tool as the next phase. The PR also includes a real before/after tmux session, not a hypothetical.

Direction: aligned. This is the planned P2 of the goal-draft design note, and Claude Code's /goal + ProposeGoal is the established analogue (its changelog shows /goal under active iteration; no direct ProposeGoal entry to cite). The two deliberate deviations — no auto mode, never replacing a running Goal — are the right conservative calls for a consent surface that starts an unattended loop.

Size: core paths touched (packages/core/src/goals, config, tools, permissions + CLI settings). Breakdown: ~352 production lines, ~339 test lines, ~14 schema, ~25 docs/skill content — under the 500-line maintainer-awareness bar. 19 files, but every one of them serves the feature (tool, registration, setting, scope restriction, skill hand-off, display names, docs). No drive-by changes spotted.

Approach: scope feels right — it matches what the design doc scoped. Reusing the generic info confirmation instead of adding a per-host dialog component is the simple path, and the tradeoff (generic button labels) is called out honestly in the PR body. Keeping the printed /goal set fallback for headless/disabled/active-Goal cases preserves every existing path.

Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:真实存在。#10002 交付的 /goal-draft 以"粘贴一行 1000 字符命令"收尾,其已合并的设计文档明确把 propose_goal 工具列为下一阶段。PR 还附了真实的 before/after tmux 记录,不是假设性问题。

方向:对齐。这是 goal-draft 设计说明里规划好的 P2;Claude Code 的 /goal + ProposeGoal 是已确立的同类方案(其 changelog 显示 /goal 在持续迭代,没有直接的 ProposeGoal 条目可引用)。两处刻意差异——没有 auto 模式、永不替换运行中的 Goal——对于"启动无人值守循环"这种同意面来说是正确的保守选择。

规模:触及核心路径(packages/core/src/goalsconfigtoolspermissions 及 CLI 设置)。拆分:约 352 行生产代码、约 339 行测试、约 14 行 schema、约 25 行文档/skill 内容——低于 500 行维护者关注线。共 19 个文件,但每个都服务于该功能本身(工具、注册、设置、作用域限制、skill 交接、显示名、文档),未发现顺手改动。

方案:范围合理,与设计文档圈定的范围一致。复用通用 info 确认而不是给每个宿主加新对话框组件,是更简单的路径;代价(按钮文案是通用的)在 PR 正文里也如实说明了。headless/禁用/已有活跃 Goal 时保留打印 /goal set 行的回退,现有路径全部保留。

风险:无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at e2c77fdae97ee70cc33ad5bd2b9ecd3f4388044b · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Code review

The approach matches what I'd have proposed independently — new tool beside get_goal / update_goal, reuse the existing info confirmation so no host needs new UI, gate registration on host capability and the opt-out setting, user-scope-only consent, skill hand-off with the text fallback kept. I verified the load-bearing claims against the base code rather than taking the PR's word for them:

  • The dialog cannot be skipped. requiresUserInteraction() is an established mechanism (used by exit_plan_mode) and I checked every bypass route in the scheduler: permissionFlow forces ask over allow rules when it is set; needsConfirmation returns true before the YOLO override; the AUTO-mode three-layer filter, the AUTO_EDIT auto-approval of info dialogs, and the PermissionRequest hook's allow path are all guarded by !requiresUserInteraction (hooks can still deny). The scheduler also sets hideAlwaysAllow, which matches the evidence showing only "Yes, allow once" / "No". Deny rules still work, as they should.
  • Registration gating is structural, not advisory. The registration sits inside registerGoalWorkerTools, which early-returns for subagent contexts; the new gate checks resolveInteractionMode !== 'headless' and the setting. Headless, subagents, and the disabled setting all miss the tool; the registration-matrix tests pin this.
  • TOCTOU handling is real. Preconditions run before the dialog and again in execute(); a replace carries expectedGoalId / expectedRevision and the goal reducer's version assert turns a concurrent /goal change into a friendly refusal instead of a clobber. Without an onConfirm approval the invocation fails closed (approved stays false).
  • Scope restriction works through the existing generic machinery. WORKSPACE_RESTRICTED_SETTINGS strips workspace values and warns; the Settings type is schema-derived so the new goals section type-checks end to end; the CLI normalizer closes the enum before it reaches core. Same pattern as agents.crossSession*.
  • renderPromptAsPlainText and the info confirmation are established surfaces (the Workflow tool uses both), so no host renders anything new.

One non-blocking nit: propose_goal isn't added to the two existing goal-tool enumerations — the chat-recording goal_runtime provenance tagging (coreToolScheduler.ts ~6294, useGeminiStream.ts ~3936/~4878) and the tool-use summarizer exclusion (useGeminiStream.ts ~5052). Practical impact is small: the tool refuses while a Goal is active, so it effectively never carries a goalContext, and the exclusion only matters because a fast-model summary could echo beside the tool's own "Goal set · …" display. Worth adding for consistency in a follow-up or a quick amend — not a merge blocker.

sequenceDiagram
    participant P1 as Model
    participant P2 as Tool scheduler
    participant P3 as ProposeGoalInvocation
    participant P4 as User dialog
    participant P5 as Goal runtime
    P1->>P2: calls propose_goal with objective
    P2->>P3: permission flow forces ask and hides always-allow
    P3->>P3: checks plan mode, trust, persistence, active Goal
    P3->>P4: info dialog with the full objective
    P4-->>P3: approve or decline
    P3->>P5: dispatch create or versioned replace
    P5-->>P3: Goal record or conflict error
    P3-->>P1: result telling it to acknowledge and end the turn
    P5->>P5: broadcast renders Goal card and starts first Goal turn
Loading
Files changed (19 of 19 shown)
File What changed
docs/design/2026-08-25-goal-draft-skill.md Design doc: adds the Phase 2 section, moves propose_goal out of Later phases
docs/users/configuration/settings.md Docs: goals.modelProposed settings table row
docs/users/features/goals.md Docs: user guide for the approval-dialog flow
packages/cli/src/config/config.test.ts CLI tests: enum normalizer
packages/cli/src/config/config.ts CLI: normalizer closes the enum before it reaches core
packages/cli/src/config/settings.test.ts CLI tests: user scope honored, workspace scope stripped with warning
packages/cli/src/config/settingsSchema.ts CLI: settings schema entry for goals.modelProposed
packages/cli/src/config/settingsUtils.ts CLI: adds goals.modelProposed to the workspace-restricted list
packages/core/src/config/config.test.ts Core tests: registration matrix (interactive, ACP, stream-json, disabled, headless)
packages/core/src/config/config.ts Core: Config param, getter, and gated lazy registration
packages/core/src/goals/goal-tools.test.ts Core tests: ProposeGoalTool behavior suite
packages/core/src/goals/goal-tools.ts Core: the ProposeGoalTool and its invocation
packages/core/src/permissions/rule-parser.ts Core: tool-name aliases for permission rules
packages/core/src/skills/bundled/goal-draft/SKILL.md Skill: tool-first hand-off with the printed-line fallback
packages/core/src/skills/bundled/goal-draft/SKILL.test.ts Skill tests: pins hand-off wording, no allowedTools grant
packages/core/src/tools/tool-names.ts Core: name and display-name constants
packages/vscode-ide-companion/schemas/settings.schema.json VS Code companion: settings schema mirror
packages/web-shell/client/components/messages/toolFormatting.ts Web Shell: display-name mapping
packages/web-shell/client/i18n.tsx Web Shell: zh tool-name string

Testing evidence (the PR's own CI — per policy this review does not run PR code)

The ubuntu unit suite and the web-shell visual capture are still running as of this review; the windows/macos unit jobs and the CLI integration job were skipped by workflow conditions (not failures). The Desktop Shell jobs on both platforms passed — those compile the whole monorepo including this diff, which is a real build signal. No red checks on this commit at review time.

Final CI results for e2c77fd (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Test (ubuntu-latest, Node 22.x) ❌ failure
Capture web-shell visuals (ubuntu-latest, Node 22.x) ✅ success
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Secret scan (TruffleHog) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

The remaining checks on the commit are bot orchestration (review-pr, label, authorize, …). Not verified: live TUI/Web Shell/ACP rendering of the dialog — the author's tmux capture below is their evidence, not an independent re-run:

Sandboxed verification would settle this: @qwen-code /tmux — that the approval dialog actually renders with the full objective under YOLO and that approving it starts the first Goal turn currently rests on the author's tmux capture (Linux only); the unit suite pins the gating logic but not host rendering. (@qwen-code /verify would settle the A/B behavioural side.) The author has write access, so a maintainer trigger is all it takes.

中文说明

代码审查

方案与我独立给出的设想一致——在 get_goal / update_goal 旁新增工具、复用现有 info 确认(三端都不需要新 UI)、按宿主能力和退出设置门控注册、同意只来自 user 作用域、skill 交接保留文本回退。我对关键断言逐一对照了基线代码:

  • 对话框无法被跳过。 requiresUserInteraction() 是既有机制(exit_plan_mode 在用),我核对了调度器里所有旁路:设置后 permissionFlow 会把 allow 规则强制为 ask;needsConfirmation 在 YOLO 豁免之前返回 true;AUTO 三层过滤、AUTO_EDIT 对 info 的自动批准、PermissionRequest hook 的 allow 分支都有 !requiresUserInteraction 守卫(hook 仍可拒绝);调度器同时设置 hideAlwaysAllow,与证据里只有 "Yes, allow once" / "No" 一致。deny 规则仍然有效。
  • 注册门控是结构性的。 注册位于 registerGoalWorkerTools 内(子 agent 上下文提前返回);新增条件检查 resolveInteractionMode !== 'headless' 与设置值。headless、子 agent、禁用设置都拿不到该工具;注册矩阵测试已固定。
  • TOCTOU 处理是真实的。 前置条件在弹窗前与 execute() 中各检查一次;replace 携带 expectedGoalId / expectedRevision,reducer 的版本断言把并发的 /goal 变更变成友好拒绝而非覆盖;未获批准时保持失败关闭。
  • 作用域限制走既有通用机制。 WORKSPACE_RESTRICTED_SETTINGS 剥离 workspace 值并告警;Settings 类型由 schema 推导,新增 goals 节类型全链路通过;CLI 规范化函数把枚举封闭后再传给 core。与 agents.crossSession* 同构。
  • renderPromptAsPlainTextinfo 确认都是既有面(Workflow 工具两者都用),宿主无需渲染任何新东西。

一个非阻塞小项: 两处既有的 goal 工具枚举没有加入 propose_goal——chat recording 的 goal_runtime 来源标记(coreToolScheduler.ts ~6294、useGeminiStream.ts ~3936/~4878)与工具使用摘要的排除列表(useGeminiStream.ts ~5052)。实际影响很小:该工具在 Goal 活跃时拒绝,基本不会携带 goalContext;排除列表只影响快速模型摘要可能与工具自身的 "Goal set · …" 展示并存。建议顺手补齐,不构成合并阻塞。

测试证据(引自 PR 自身的 CI——按策略本审查不运行 PR 代码)

截至审查时 ubuntu 单测与 web-shell 视觉截图仍在运行;windows/macos 单测与 CLI 集成测试被工作流条件跳过(不是失败)。两平台的 Desktop Shell 任务已通过——它们连同本 diff 编译整个 monorepo,是真实的构建信号。审查时该提交无红色检查。上方表格区域会由 finalize 流程在 CI 落定后就地更新。

未验证:TUI/Web Shell/ACP 对话框的实际渲染——作者在 Linux 上的 tmux 记录是其自述证据,不是独立复跑。维护者可触发 @qwen-code /tmux(TUI 面)或 @qwen-code /verify(A/B 行为面)来闭环;作者有 write 权限,维护者直接触发即可。

Qwen Code · qwen3.8-max

Reviewed at e2c77fdae97ee70cc33ad5bd2b9ecd3f4388044b · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean, minimal, direction pre-approved in the merged design doc; the only open items are the non-blocking enumeration nit and CI still in flight.

Stepping back: this is exactly the follow-up phase the goal-draft design note promised, and it lands the hard parts right. My independent proposal for this feature and the PR's implementation converge on the same shape, and where the PR goes further — the versioned replace, the double precondition check across the dialog window, the closed-enum normalizer, the workspace-scope lockdown — each addition earns its place. Nothing in the diff is unrelated, nothing feels over-engineered, and the security model (consent for starting an autonomous loop must come from a human at a dialog that no rule or mode can skip) verifies cleanly against the scheduler code, not just against the PR's description. The test suite pins the behaviors that matter rather than the implementation details.

Reservations, plainly stated: the cross-host rendering of the dialog (TUI buttons, Web Shell description-only approval, ACP) is covered by unit tests and the author's tmux session but not independently exercised here, and the ubuntu unit suite had not finished when this review ran. Neither is a reason to hold the PR — they are what the green-CI precondition and the sandboxed lanes above are for.

Verdict: approve, with approval deferred until CI lands green on e2c77fdae97ee70cc33ad5bd2b9ecd3f4388044b (the ubuntu unit suite and web-shell visuals are still in flight). If anything lands red, the finalize step flags it instead of approving.

中文说明

置信度:4/5 —— 干净、最小化,方向在已合并的设计文档中已预先确认;唯一未决项是上面那个非阻塞枚举小项,以及 CI 仍在运行。

退一步看:这正是 goal-draft 设计说明承诺的后续阶段,而且难点都做对了。我独立给出的方案与 PR 的实现收敛到同一形状;PR 更进一步的地方——带版本的 replace、跨对话框窗口的双重前置条件检查、封闭枚举的规范化、workspace 作用域锁定——每一处都物有所值。diff 中没有无关改动,没有过度设计;安全模型(启动无人值守循环的同意必须来自对话框里的真人,任何规则或模式都不能跳过)在调度器代码层面得到验证,而不只是 PR 描述里的说法。测试套件固定的是行为而非实现细节。

坦率地说保留意见:对话框的跨宿主渲染(TUI 按钮、Web Shell 仅描述批准、ACP)由单测和作者的 tmux 记录覆盖,但这里没有独立复跑;审查时 ubuntu 单测尚未结束。这两点都不是扣住 PR 的理由——绿色 CI 前置条件和上面的沙箱通道正是为此而设。

结论:批准,但批准延迟到 CI 在 e2c77fdae97ee70cc33ad5bd2b9ecd3f4388044b 上全绿后生效(ubuntu 单测与 web-shell 视觉仍在运行)。若有红色结果,finalize 环节会标记而不是批准。

Qwen Code · qwen3.8-max

Reviewed at e2c77fdae97ee70cc33ad5bd2b9ecd3f4388044b · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 5c640ce. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

terminal-turn-error-copy-narrow-dark before/after

terminal-turn-error-copy-narrow-light before/after

Full-resolution recordings (.webm) are attached to the workflow run.

Qwen Code · web-shell visuals

@qqqys

qqqys commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

已修复 ProposeGoal 的 CLI 工具名翻译覆盖,补齐 en、zh、zh-TW。验证:CLI i18n 31/31、全仓 typecheck、完整 build/bundle 均通过。

qqqys added 2 commits August 27, 2026 11:32
… turn ends

The first end-to-end run set the Goal from inside propose_goal's execute().
That works for the Goal but not for the turn that proposed it: with a Goal
now active, the tool-result continuation of that turn has no Goal permit
and client.ts rejects it with "An active Goal requires an exact turn
permit", which surfaces as an error card before the runtime's own Goal
turn takes over.

The tool now parks the approval on Config (setPendingGoalProposal) and the
client applies it at the boundary a typed /goal set takes effect: once the
model has stopped calling tools, before Stop hooks (settlePendingGoalProposal
-> applyPendingGoalProposal). The runtime's broadcast then renders the Goal
card and starts the first Goal turn right after the model's acknowledgement.
An approval still parked when the next real user query starts belongs to a
cancelled turn and is discarded; a Goal that became active meanwhile is never
replaced, and a moved expected version is reported as a conflict instead of
thrown.

Claude-Session: https://claude.ai/code/session_01FV7i3w7egJ2kMw4AhQC38Z
qqqys added a commit to qqqys/qwen-code that referenced this pull request Aug 27, 2026

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — the 'Integration Tests (CLI, No Sandbox)' check was skipped in CI and its suite did not run locally.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 5)": none — the walk completed inside budget; no check was cut short..

中文说明

仅完成部分审查,审查缺口已披露。

未审查:build-and-test — the 'Integration Tests (CLI, No Sandbox)' check was skipped in CI and its suite did not run locally。

未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 5)"none — the walk completed inside budget; no check was cut short.

— qwen3.8-max via Qwen Code /review (v0.22.2)

Comment thread packages/core/src/core/client.ts Outdated
Comment thread packages/core/src/core/client.ts Outdated
Comment thread packages/core/src/config/config.ts
Comment thread packages/core/src/goals/goal-tools.ts
Comment thread packages/core/src/core/client.ts Outdated
Comment thread packages/cli/src/config/config.ts
Comment thread packages/core/src/core/client.ts Outdated
Comment thread packages/core/src/core/client.ts Outdated
Comment thread packages/core/src/config/config.ts
Comment thread packages/core/src/goals/goal-tools.ts Outdated

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed. Suggestions are inline.

5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • R1-7 blocker() readiness gate — still stands (code unchanged), already reported (comment 3869318070)
  • R1-8 last-write-wins parking slot — still stands (code unchanged), already reported (comment 3869318076)
  • R1-9 approvedAt write-only field — still stands (code unchanged), already reported (comment 3869318082)
  • R1-10 replacedGoalId unread field — still stands narrowed to replacedGoalId (the goal field is now read by the pause rollback), already reported (comment 3869318103)
  • R1-11 five untested propose_goal lifecycle behaviors — all 5 locations still stand (cli wiring, entry abort branch, park gate, runtime-replacement clear, execute re-check), already reported (comments 3869318110, 3869318115, 3869318123, 3869…

Not reviewed: build-and-test — the 'Integration Tests (CLI, No Sandbox)' check was skipped in CI and its suite did not run locally.

Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.

Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:

  • packages/core/src/config/config.ts:7459 — [review] Config park/take pair never exercised by any test; the take-as-peek mutation (removing the clear inside takePendingGoalProposal) ships the scoped suite 663/663 green — anchored on code unch…
中文说明

仅完成部分审查,审查缺口已披露。 建议见行内评论。

本轮确认的 5 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

未审查:build-and-test — the 'Integration Tests (CLI, No Sandbox)' check was skipped in CI and its suite did not run locally。

未审查:反向审计——在 5 轮的反审轮数上限内未收敛。

收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.22.2)

Comment thread packages/core/src/core/client.ts Outdated
Comment thread packages/core/src/core/client.ts Outdated
Comment thread packages/core/src/core/client.ts Outdated
Comment thread packages/core/src/config/config.ts
Comment thread packages/core/src/config/config.test.ts Outdated

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed. Suggestions are inline.

Not reviewed: build-and-test — the 'Integration Tests (CLI, No Sandbox)' check was skipped in CI and its suite did not run locally.

Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:

  • packages/core/src/goals/goal-tools.test.ts:1878 — [probe] applyPendingGoalProposal's persistence-failure catch branch has no test
中文说明

仅完成部分审查,审查缺口已披露。 建议见行内评论。

未审查:build-and-test — the 'Integration Tests (CLI, No Sandbox)' check was skipped in CI and its suite did not run locally。

收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.22.2)

Comment thread docs/users/features/goals.md Outdated
Comment thread packages/core/src/goals/goal-tools.test.ts
Comment thread packages/core/src/core/client-goal.test.ts
qqqys added 2 commits August 29, 2026 13:41
… uncovered

Adds the tests round 3 asked for, each verified to go red under the
mutation it guards against:

- goal-tools: execute() refuses when the set-once parking slot rejects a
  second approval that passed the pending re-check (deleting the branch
  fails it).
- client-goal: "cancellation during runtime readiness" now asserts the
  proposal was taken and the slot is empty, not only that nothing was
  dispatched.
- core config: propose_goal stays unregistered in interactive+stream-json
  and interactive+Zed sessions (isInteractive() true, mode 'acp'), so the
  gate cannot silently regress to isInteractive(); startNewSession clears
  a parked approval; take hands it out exactly once.
- cli config: loadCliConfig wires goals.modelProposed through (default
  alwaysAsk, disabled, unknown value), red when the wiring line is dropped.

goals.md no longer claims Web Shell has the tool; Web Shell and other
ACP-driven sessions keep the printed /goal set hand-off until they have a
turn-boundary settlement path. Also fixes the LlmEventType rename that
merging main introduced into client-goal.test.ts.
@qqqys

qqqys commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator Author

Merged main (103 commits; the only conflict was the settings table row — resolved by re-inserting the row into main's table, net diff still +1 line) and closed out the review rounds:

  • Every R1 / R3 thread now has a reply pointing at the commit and test that covers it, and is resolved. Mutation-checked today against client-goal.test.ts: commenting out any of the three tail settle sites, the UserQuery-entry discard, or the !turnEnded gate turns the suite red.
  • 751799f adds the tests round 3 asked for (set-once refusal branch, consumed-slot assertion on abort, interactive+stream-json / interactive+Zed registration divergence, runtime-rebuild clears the slot, loadCliConfig wiring for goals.modelProposed), each verified red under its mutation, and fixes the LlmEventType rename that merging main introduced into the bot's test.
  • Docs: goals.md no longer claims Web Shell has propose_goal; Web Shell / ACP keep the printed /goal set hand-off until they have a turn-boundary settlement path (follow-up). The PR body's registration and settlement paragraphs are updated to match HEAD.

Verification at 751799f: core tsc --noEmit clean; core client-goal + goal-tools + config 678 tests, cli config (goals.modelProposed + normalize) 4 tests; full npm run build green.

中文说明

已合并 main(103 个提交;唯一冲突是设置表那一行,把该行重新插入 main 的表后净差异仍为 +1 行),并收尾各轮评审:

  • R1 / R3 的每条线程都已回复(指向覆盖它的提交与测试)并 resolve。今天对 client-goal.test.ts 做了变异验证:注释掉三处尾部 settle 调用点中的任意一处、UserQuery 起始的丢弃、或 !turnEnded 门,测试都会变红。
  • 751799f 补齐第三轮要求的测试(set-once 拒绝分支、中止时槽已被取空的断言、interactive+stream-json / interactive+Zed 的注册分叉、runtime 重建清空暂存槽、loadCliConfiggoals.modelProposed 的接线),每条都验证过在对应变异下变红;并修复合并 main 带进 bot 测试的 LlmEventType 改名。
  • 文档:goals.md 不再声称 Web Shell 有 propose_goal;Web Shell / ACP 在拥有 turn 边界应用路径之前继续使用打印 /goal set 行的交接(后续跟进)。PR 正文的注册与应用段落已与 HEAD 对齐。

751799f 上的验证:core tsc --noEmit 干净;core client-goal + goal-tools + config 678 个测试、cli config(goals.modelProposed + normalize)4 个测试通过;全量 npm run build 通过。

@qqqys

qqqys commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Aug 29, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • applyPendingGoalProposal's GoalPersistenceUnavailableError catch clause has no test — already recorded as the round-3 deferral (review 5042138776)

Not reviewed: build-and-test — the 'Integration Tests (CLI, No Sandbox)' check was skipped in CI and its suite did not run locally.

Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:

  • packages/core/src/core/client-goal.test.ts:778 — [probe] D4-1 next-query discard test cannot detect removal of client.ts:3137 (cap-path finally consumes the proposal)
  • packages/core/src/core/client.ts:848 — [probe] D4-2 approved proposal that cannot be applied at turn end is dropped with a debug log only; user and model never told
  • packages/core/src/core/client.ts:848 — [probe] D4-3 settle's !runtime guard has no crossing test (tsc catches a naive removal, but graceful-drop semantics are unpinned)

Convergence: round 4 posted 6 inline comment(s), 6 of them reported for the first time; the previous round posted 3 (3 new). Findings keep coming back to the same files: packages/core/src/core/client-goal.test.ts (findings in round 3; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

仅完成部分审查,审查缺口已披露。

本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

未审查:build-and-test — the 'Integration Tests (CLI, No Sandbox)' check was skipped in CI and its suite did not run locally。

收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。

收敛情况:第 4 轮发布了 6 条行内评论,其中 6 条是首次提出;上一轮发布了 3 条(其中 3 条首次提出)。发现反复回到同一批文件:packages/core/src/core/client-goal.test.ts(第 3 轮已出过发现,本轮又有 1 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。(仅为观察——本轮评审未因此扣留任何内容。)

— qwen3.8-max via Qwen Code /review (v0.22.3)

Comment thread packages/core/src/core/client.ts Outdated
Comment thread packages/core/src/core/client.ts Outdated
Comment thread packages/cli/src/config/config.test.ts
Comment thread packages/core/src/config/config.test.ts Outdated
Comment thread packages/core/src/config/config.test.ts Outdated
Comment thread packages/core/src/core/client-goal.test.ts
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 AutoFix hit a verification-gate error before reaching a verdict (attempt 11/100) — it will retry on the next scan.

⚠️ This change was NOT pushed — any commit referenced below was made only in the runner workspace and has been discarded. What the agent reported:
This round (head 9f1df49b8f): merged origin/main as requested (--conflict true), verified and declined the remaining Critical with production-runtime evidence, and addressed the two non-blocking observations from @wenshao's round-2 verification.

Base conflict resolution (--conflict true)

Merged origin/main (41 commits). One conflict, in the docs/users/configuration/settings.md settings table:

  • origin/main updated the tools.eager wording (test(core): pin tools.eager Object.prototype-key tolerance and correct related docs #10479) and re-padded the table; this PR carries the goals.modelProposed row. Resolved by keeping main's table and re-inserting the goals.modelProposed row at its position (after tools.workflowsEnabled) — the same approach used when this table conflicted in the earlier merge.
  • All other files (including client.ts, client-goal.test.ts, config.ts) auto-merged cleanly; the goal code was untouched by incoming commits.

Feedback dispositions

R9-1 (Critical, rc:3890179439) — Declined with probe evidence

The round-9 finding claims the goal-permit early-return path can exit with an approved proposal still parked, because pause does not bump the revision so the permit "remains valid under isCurrentPermit". I re-ran the exact chain against the production createGoalRuntime (not the mock runtime): Goal active → UserQuery admitted with a beginTurn permit → dispatch({ action: 'pause' }) mid-turn → approval parked under the turn's prompt id → turn ends with no tool calls. Probe report:

中文说明

🤖 AutoFix 在得出结论之前遇到验证门错误(第 11/100 次尝试)—— 将在下次扫描时重试。

Run log: https://github.com/QwenLM/qwen-code/actions/runs/33424919148


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

wenshao
wenshao previously approved these changes Aug 31, 2026
@wenshao

wenshao commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /resolve

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 AutoFix hit a verification-gate error before reaching a verdict (attempt 12/100) — it will retry on the next scan.

⚠️ This change was NOT pushed — any commit referenced below was made only in the runner workspace and has been discarded. What the agent reported:

Round summary — PR #10171 (Critical-only window, conflict round)

Critical-only mode is active (window seeded at round 9, round counter ≥ 5). This round: merged origin/main (requested conflict resolution), disposed of the one new Critical (R9-1), re-verified the standing Criticals fixed by earlier commits, and addressed the two non-blocking observations from the maintainer's round-2 E2E (the only human feedback still in budget).

Base-conflict resolution (required, --conflict true)

Merged origin/main @ bd59085311 (42 commits) as 11e5145cb1. Exactly one conflict: the #### tools table in docs/users/configuration/settings.md — main re-flowed the table padding while this PR carries the added goals.modelProposed row. Resolved by understanding both sides: kept main's table verbatim and re-inserted the goals.modelProposed row (net +1 line, same resolution shape as the previous main-sync), then normalized with prettier. All code files auto-merged; the notable upstream overlaps (output-style turn reminder #10282 in client.ts/client-goal.test.ts, Goal token budget #10543 in the config files, extension skill state #10600) landed beside this PR's hunks and are covered by the verification below.

Feedback dispositions

R9-1 (Critical, rc:3890179439) — Declined as refuted (probe-verified). The claimed chain requires finishTurn(goalPermit) to succeed on the goal-permit early-return path after a mid-turn Goal pause. A probe against the real `create

中文说明

🤖 AutoFix 在得出结论之前遇到验证门错误(第 12/100 次尝试)—— 将在下次扫描时重试。

Run log: https://github.com/QwenLM/qwen-code/actions/runs/33440419108


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

Qwen Code attempted to resolve merge conflicts but the run did not complete successfully.

Check the workflow run for full logs.

@wenshao

wenshao commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /resolve

@qqqys qqqys removed the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Sep 1, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 AutoFix ran out of time before finishing (timeout (3600000ms)) (attempt 13/100) — it will retry on the next scan.

⚠️ This change was NOT pushed — any commit referenced below was made only in the runner workspace and has been discarded. What the agent reported:
Qwen failed during address-review: timeout (3600000ms).

See the Qwen Autofix agent step logs for model/tool output.

中文说明

🤖 AutoFix 在完成前耗尽了时间(timeout (3600000ms))(第 13/100 次尝试)—— 将在下次扫描时重试。

Run log: https://github.com/QwenLM/qwen-code/actions/runs/33452214748


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

Qwen Code resolved the merge conflicts and pushed the branch update. The head branch moved to 9d54eab868778e5bc6bbd35fed05e15311305d62 while the conflicts were being resolved; the resolution was replayed on top of it and re-checked before the push — where a file still conflicted and the new commits had not touched it, the replay took the agent's version. The pushed branch is that replay: its diff against main is attached as the qwen-resolve-pr-10171-pushed artifact on the workflow run; the qwen-resolve-pr-10171-attempt-1 artifact describes the original resolution it was replayed from.

Merge resolution — PR #10171 ← upstream/main

Root cause. Main's #10479 (f86098363d) rewrote every row of the #### tools table in docs/users/configuration/settings.md — correcting the tools.core description (deny semantics for MCP tools) and tools.eager (two carve-outs for resumed sessions / tools.visible) — which widened the Description column and re-padded all 24 rows. This PR inserts one new row (goals.modelProposed) into the middle of that same table. Both edits hit the same contiguous table block, so git could not auto-merge.

Textual, not semantic. The sides modified no shared row: main re-padded/edited existing rows, the PR only added one. Everything outside the table auto-merged. Resolution = keep main's table intact, re-insert the PR's row at the author's chosen position, re-padded to the new column widths:

| `goals.modelProposed` | enum | Controls the `propose_goal` tool … | `alwaysAsk` | User, System, and SystemDefaults scopes … |

(abbreviated; the real row is space-padded to 1823 chars like every other table row)

What is load-bearing.

  1. Row order: goals.modelProposed sits between tools.workflowsEnabled and tools.truncateToolOutputThreshold — where the PR author placed it. This doc has no #### goals section; the setting lives in the tools table because it controls a tool.
  2. Padding invariant: every table row is exactly uniform in width (now 1823 chars; 1643 before main's change); prettier enforces this. The Description column is padded to 1428 chars, dictated by main's lengthened tools.core description. A row inserted at the old width would be rewritten by the next format run.
  3. tools.core / tools.eager descriptions must stay at main's wording; the PR never touched those cells.

Could not verify. No build/typecheck/tests run (out of scope). Only the conflicted file was modified. Cross-checks done: all 8 files referencing modelProposed (settingsSchema.ts, both packages' config.ts + config.test.ts, settingsUtils.ts, settings.test.ts, settings.schema.json) auto-merged and carry the PR's references in identical counts; the merged diff vs main contains only the one added row for this file. Doc-only change — no caller or test depends on it.

中文说明

根本原因。 main 分支的 #10479f86098363d)重写了 docs/users/configuration/settings.md#### tools 表格的全部行——修订了 tools.coretools.eager 的描述,使 Description 列变宽并重新填充了全部 24 行;而本 PR 在同一表格中间插入了一行新设置(goals.modelProposed)。两处编辑落在同一连续表格块上,git 无法自动合并。(main 的 #10543 也改了此文件,但位于其他章节,已干净合并。)

纯文本冲突,非语义冲突。 双方没有修改同一行。解决方式:完整保留 main 的表格,把 PR 新增的行按作者选定的位置(tools.workflowsEnabledtools.truncateToolOutputThreshold 之间)重新插入,并按新列宽重新填充空格。

关键约束。

  1. 行序:该行必须保持在上述两条 tools.* 行之间——文档没有 #### goals 小节,此设置因控制工具而放在 tools 表内。
  2. 填充不变量:表格每行宽度一致(现 1823 字符),prettier 会强制对齐;Description 列宽 1428,由 main 加长后的 tools.core 描述决定。
  3. tools.core / tools.eager 的描述必须保留 main 版本,PR 未改动这两处。

未能验证的部分。 未运行构建或测试(超出本任务范围),仅修改了冲突文件。已核对:所有引用 modelProposed 的 8 个代码文件均为自动合并,PR 侧引用数量完整保留;相对 main 的最终差异仅为该文件中新增的一行。纯文档变更,不影响任何调用方或测试。

@qwen-code-dev-bot qwen-code-dev-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at head b69c986c.

  • The two later commits after the round-9 review are upstream merges only; the goal files (goal-tools.ts, goal-runtime.ts, client.ts proposal paths) are unchanged since the last findings round. 40/40 threads are resolved.
  • The round-8 Criticals were both recorded by the reviewer itself as deferred non-blockers (fails-closed, new surface). Re-checked against the code at this head: the turn-key binding drops the proposal when a new user/retry chain starts or a turn exits abnormally, only the owning prompt_id frame may apply, and dispatch failure yields a applied:false reason rather than an active-but-unannounced Goal. The plan-mode / untrusted / active-goal preconditions are checked before the dialog and again in execute() before parking.
  • Consent posture looks right: requiresUserInteraction() blocks rule/YOLO/AUTO_EDIT auto-approval, goals.modelProposed is workspace-restricted so a repo cannot enable it, and headless/non-interactive sessions never register the tool.
  • CI on this head has no failures (Test/Integration/security checks green; the review pipeline is still running); per the channel convention CI is outside my gate and this call is on the review itself.

yiliang114
yiliang114 previously approved these changes Sep 1, 2026

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review passed.

  • Consent gate is airtight: requiresUserInteraction() = true + default permission ask, and the scheduler honors that flag at every allow-rule / AUTO path, so no allow rule, YOLO, or AUTO_EDIT can skip the dialog. Preconditions are checked before the dialog and re-checked in execute().
  • The parked-approval lifecycle is handled exhaustively in client.ts: settlement only at the proposing turn's true end (matched by prompt_id, kept parked through tool-result and tail-hook continuations), drop on abort/new user query/retry, discard on non-normal completion in the finalizer, and pause-if-aborted-during-dispatch. Config.startNewSession clears a stale approval.
  • Registration is correctly gated: interactive sessions only; not registered under ACP, stream-json, headless, or goals.modelProposed: disabled; subagent cores are non-interactive by construction.
  • goals.modelProposed is workspace-restricted (repo cannot enable the tool), unknown values normalize to the default, and the vscode schema / i18n / web-shell display names are all updated.
  • Verified locally: goal-tools + client-goal + goal-draft skill tests 111/111; cli settings/config modelProposed tests pass; core Config registration/parking tests pass.
  • CI: ubuntu Test failure is a runner-overload timeout cascade in files this PR does not touch, ending in a job-level cancel; web-shell E2E Smoke is a known main-level flake.

@qqqys
qqqys dismissed stale reviews from yiliang114 and qwen-code-dev-bot via 5c640ce September 1, 2026 06:48

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review after the new push: the only changes since my previous approval are upstream/main merges (which now include the already-merged #10283); the feature diff is unchanged and the settings schema merge correctly keeps both goals.modelProposed and general.outputStyle. Previous verification still applies:

  • Consent gate: requiresUserInteraction + default ask, no allow rule / YOLO / AUTO_EDIT can skip the dialog; preconditions re-checked in execute().
  • Parked approval lifecycle covered at every client.ts exit path, bound to the proposing turn's prompt_id.
  • Registration gated to interactive sessions only; workspace-restricted setting; i18n / web-shell / vscode schema updated.
  • Local tests passed on the reviewed head: goal-tools + client-goal + goal-draft skill 111/111, cli config/settings modelProposed tests, core Config registration/parking tests.

@qwen-code-dev-bot qwen-code-dev-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at head 5c640ce0 (my earlier approval on b69c986c was auto-dismissed by the two later upstream merges).

  • Verified the increment is merge-only: the two commits are upstream syncs; the propose_goal files are untouched, and the drift on shared files (config.ts, settingsSchema.ts, client.ts) comes from already-merged upstream PRs. The turn-settlement logic still reads intact at this head (13 settle/takePendingGoalProposal sites), and 40/40 threads remain resolved.
  • CI on this head has no failures so far (Test and the review pipeline still running); per the channel convention the call is on the review itself.
  • Note: the repo's review decision may still display CHANGES_REQUESTED from the older bot round whose deferred items were all test/doc follow-ups; those were re-checked at b69c986c and are unchanged here.

@qqqys
qqqys enabled auto-merge September 1, 2026 07:44
@qqqys
qqqys added this pull request to the merge queue Sep 1, 2026
Merged via the queue into QwenLM:main with commit 56f92c8 Sep 1, 2026
52 of 54 checks passed
pull Bot pushed a commit to bit-cook/qwen-code that referenced this pull request Sep 1, 2026
…AME (QwenLM#10671)

* fix(autofix): clamp gate test load explicitly instead of via RUNNER_NAME

The verification gate launches through an env -i allowlist that drops
RUNNER_NAME, so the vitest configs' ECS load clamps (60s test/hook
timeouts, maxWorkers 25%) silently deactivate inside the gate: tests run
with 15s timeouts, unbounded workers and coverage collection on a host
shared with other autofix jobs. Under pool saturation this produced both
false rejections (QwenLM#10171 round 3: 73 load-induced 15s timeouts in files
the PR never touched, charged to the round) and gate deaths past the
step's 60-minute cap that discarded verified fixes ("verification-gate
error": QwenLM#10171 rounds 1/2/5-7, QwenLM#10543 five in a row).

Pass the clamp values explicitly on both gate vitest invocations (the
per-package --changed run and the bite check) so the verdict does not
depend on env plumbing or runner naming, and disable coverage: nothing
in the gate consumes it, and its collection dominated the overrun
(72,000 CPU-seconds of collect in one 1,560s gate leg).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AhZA7LdQXZjcjfiPsoZkqZ

* fix(autofix): clamp the gate's third vitest leg and pin the clamps

Addresses review round 1 on QwenLM#10671.

R1-6 The contracts check runs a web-shell vitest from inside the gate's
own `env -i` child, and web-shell's config sets no timeouts at all, so
that leg ran at vitest's 5s default on the same saturating host — the
false-rejection class this PR removes, surviving in a sibling path. The
clamp array moves above the contracts call and is handed to the shared
script through AUTOFIX_VITEST_FLAGS; the issue-fix gate invokes the same
script where RUNNER_NAME is present and leaves the variable unset, so its
invocation is unchanged.

R1-3 Nothing pinned that the clamps reach any invocation — every existing
assertion is a prefix that ends before the expansion, so dropping it from
a leg (or emptying the array, silent without `set -u`) stayed green while
the gate reverted to 15s timeouts and coverage on. Three structural pins
added on the review runner only, plus a contracts-script case that runs
with AUTOFIX_VITEST_FLAGS set and asserts the flags reach npm.

R1-4 The array hand-copies the ECS branch of three vitest configs, and
inside the gate the CLI flags outrank the config — so raising an ECS
ceiling to shelter a heavier test would leave the gate enforcing the old
one and rejecting a fix that is green in normal CI. A parity test in
scripts/tests/unit-vitest-configs.test.ts re-imports core, cli and
acp-bridge under a stubbed ecs-qwen RUNNER_NAME (they read the env at
import time) and asserts equality with the array parsed out of the shell
script.

R1-5 Narrowed the comment's claim, per the finding's own minimum. The
residual is real and now named in the script: a handful of test files set
their ceiling with a runtime `vi.setConfig` keyed on RUNNER_NAME, which
outranks the CLI, so they keep their non-ECS values in here. Closing it
needs a gate sentinel on both env -i allowlists plus a change in each
file — a separate slice, not folded into this one.

R1-1 is declined; see the thread. Its two premises did not reproduce
against the lockfile-pinned vitest 1.6.1 under packages/sdk-typescript:
the full suite passes with --maxWorkers=25% (37 files, 1747 tests,
exit 0), and --maxThreads is rejected by 1.6.1 and 3.2.7 alike, so
--maxWorkers is the spelling both majors accept rather than neither.

* fix(autofix): pin the clamp witnesses and correct the unclamped-leg record

Addresses review round 2 on QwenLM#10671; re-verifies round 1's fixes with
mutation probes.

R2-1 The comments justifying the unclamped issue-fix leg rested on a
premise that does not hold for that leg: web-shell's vitest config sets
no timeouts and has no RUNNER_NAME branch, so the drift test runs at
vitest's 5s default wherever it runs. Corrected at all four mirror
sites (both scripts and both test comments): the review gate passes
explicit clamps; the issue-fix gate and repo-hygiene's docker leg —
the previously unnamed third caller — invoke the contracts script
without the variable and accept the 5s default. The alternative
(exporting AUTOFIX_VITEST_FLAGS in the issue-fix gate step) edits a
workflow file this PR has never touched and stays out of scope.

R2-2 --maxWorkers=25% is coerced to NaN by vitest 1.x; the
lockfile-pinned 1.6.1 under packages/sdk-typescript survives only
because its config sets a numeric poolOptions.threads.maxThreads,
which tinypool reads before ctx.config.maxWorkers. Pin the shield:
a new case derives vitest-1.x workspaces from nested lockfile copies
and asserts each keeps the threads pool and a numeric maxThreads,
failing with a directive if such a workspace is missing from the
config registry. Mutation-verified red on shield removal.

R2-3 The export is the only line carrying the clamps across the
process boundary into check-autofix-contracts.sh; nothing pinned it.
Added the structural pin plus an ordering assertion against the
contracts call — deleting the export or moving it below the call now
fails the suite. Both mutants verified red.

R2-4 The contracts case's fake npm logged $*-joined argv, rendering a
joined-blob flag byte-identically to separate words; the [*]-for-[@]
mutant survived. The shim now logs one bracketed line per argv word
and the four expectations in the case were updated; the mutant now
fails.

R1-3/R1-4/R1-5/R1-6 (round-2 commit) re-verified with mutation
probes: dropping either invocation's expansion, dropping the
assignment, emptying the array, drifting --testTimeout to 61000, and
dropping the flag expansion inside the contracts script each turn an
existing witness red.

R1-1 remains declined: the deterministic crash does not reproduce at
this head (the leg passes with sdk's shield present), but the round-2
rationale was wrong and is corrected on the thread; the residual risk
is the shield R2-2 now pins.

* test(autofix): pin both sides of the gate's AUTOFIX_VITEST_FLAGS transport

Addresses review round 3 on QwenLM#10671; both pins mutation-verified.

R3-1 Nothing pinned the VITEST_LOAD_CLAMPS definition above its
consumers: the existing pins are position-blind (toContain here, the
parity regex in unit-vitest-configs.test.ts matches anywhere), so
moving the array below its consumers left every pin green while bash
expanded the then-unset array to zero words under the gate's
`set -eo pipefail` without `-u` — AUTOFIX_VITEST_FLAGS goes empty and
the package and bite legs lose all four clamps, silently reverting to
the incident conditions. Added an explicit ordering pin against the
star-join, the first consumer in script order, which pins the
definition above every consumer. Outright deletion was already caught
by the parity test's existence assertion; the move was the only
surviving hole. Move mutant verified red (61668 < 34052 fails).

R3-2 The remove side was pinned nowhere: moving `unset
AUTOFIX_VITEST_FLAGS` above the contracts call (or deleting it)
strips the export the drift leg inherits at child-spawn time, leaving
the web-shell drift test at vitest's 5s default with every
establish-side pin green. Added the symmetric ordering pin, contracts
call before unset. Move and delete mutants both verified red
(34333 < 34241 and 34306 < -1 fail).

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: 易良 <1204183885@qq.com>
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.23.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants